Fix17713 - Reverting PR - 17649 - Make the interaction between #line and #nowarn directives consistent #17724

KevinRansom · 2024-09-13T16:48:53Z

This language feature caused bootstrapping issues for this repo introducing additional unintended warnings when building the F# bootstrapper with F# 9.0.

This was first noticed by the sourcebuildteam who have a strict bootstrapping requirement for delivery to Linux distributions.

Additionally the langversion:8 implementation produced different error output from the 8.0.* version of the compiler.

The way that we use #line and # n filename in the product is quite extensive and fragile. It is clear that we need a more robust solution highlighted by the issue the original PR was intended to solve, this has highlighted that we do need to address the consistency issue. Everyone on the team agreed that the current implementation that we are going back to is not ideal, but this reversion allows us to identify a better design for a fix.

The warnings by bootstrapping with F# 9.0 produced are:

##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(35,39): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_decl has been constrained to be type 'ParserSpec -> ParserSpec'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(38,107): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_idents has been constrained to be type 'Identifier list'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(38,107): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_idents has been constrained to be type 'Identifier list'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(46,42): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_rule has been constrained to be type 'Identifier * Rule list'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(46,42): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_rule has been constrained to be type 'Identifier * Rule list'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(47,69): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_clauses has been constrained to be type 'Rule list'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(47,69): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_clauses has been constrained to be type 'Rule list'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fs(411,20): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_optbar has been constrained to be type 'unit'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fs(411,20): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_optbar has been constrained to be type 'unit'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fs(431,20): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_optsemi has been constrained to be type 'unit'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fs(431,20): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_optsemi has been constrained to be type 'unit'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(50,51): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_clause has been constrained to be type 'Rule'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(50,51): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_clause has been constrained to be type 'Rule'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(51,55): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_syms has been constrained to be type 'Identifier list'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(51,55): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_syms has been constrained to be type 'Identifier list'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(51,58): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_optprec has been constrained to be type 'Identifier option'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fsy(51,58): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype_optprec has been constrained to be type 'Identifier option'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]
 /vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fs(223,20): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype__startspec has been constrained to be type 'obj'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj] [/vmr/artifacts/source-built-sdks/Microsoft.DotNet.Arcade.Sdk/tools/Build.proj]
##[error]/vmr/src/fsharp/buildtools/fsyacc/fsyaccpars.fs(223,20): error FS0064: This construct causes code to be less generic than indicated by the type annotations. The type variable 'gentype__startspec has been constrained to be type 'obj'. [/vmr/src/fsharp/buildtools/fsyacc/fsyacc.fsproj]

github-actions · 2024-09-13T16:49:30Z

⚠️ Release notes required, but author opted out

Warning

Author opted out of release notes, check is disabled for this pull request.
cc @dotnet/fsharp-team-msft

docs/release-notes/.Language/preview.md

vzarytovskii · 2024-09-13T17:11:45Z

FYI @Martin521, it was blocking our sourcebuild pipeline (Kevin tried to fix it yesterday, but no luck apparently).

… fix17713

Martin521 · 2024-09-15T20:33:56Z

FYI @Martin521, it was blocking our sourcebuild pipeline (Kevin tried to fix it yesterday, but no luck apparently).

How can I reproduce this?
I am not sure what "bootstrapping with F# 9.0" exactly means.
It might make sense to integrate at least the changes to fsyaccdriver.fs and the *.fsy files to avoid the same problem next year.

KevinRansom · 2024-09-16T03:39:44Z

@Martin521 Build the bootstrapper with an F# 9.0 compiler with these changes.

Martin521 · 2024-09-16T07:58:49Z

Ok, if that is a requirement, then this fix can never be done in one step.
It is important then, to already now insert the extra #nowarn directives into the grammar files.
This doesn't hurt, does not change the behavior or output, but enables us next year to create a fix.

EDIT: It can be done in one step, with just one additional #nowarn in fsyaccpars. See below.

Martin521 · 2024-09-16T08:36:32Z

#nowarn "64" should be provisionally inserted in fsyaccpars.fs after line 15.
I will create a PR in the FsLexYacc repository to change the upstream source fsyaccpars.fsy to the same effect.

Martin521 · 2024-09-16T08:51:40Z

Actually, with just this additional #nowarn in fsyaccpars, the original fix (#17649) will work also in F# 9 bootstrapping.
So, up to the team if the fix should happen now or next year.

KevinRansom · 2024-09-16T19:13:11Z

@Martin521 we are going with the reversion for now, we need to have a fuller discussion on how we want nowarn to work if we decide to change it.

Here is my discussion of why I believe reversion is the way to go right now: #17748 (comment)

Martin521 · 2024-09-16T19:30:48Z

The "scoped nowarn" RFC discussion thread might be an appropriate place for a fuller discussion.

vzarytovskii · 2024-09-16T20:02:19Z

The "scoped nowarn" RFC discussion thread might be an appropriate place for a fuller discussion.

Yeah, we should continue there. @KevinRansom @T-Gro we should discuss it on some of the upcoming triage sessions

ellahathaway · 2024-09-16T22:40:38Z

Please make sure that this flows into .NET 9.0. I believe fsharp's refs/heads/release/dev17.12 branch flows to .NET 9.0 release branch. Thanks in advance :)

ellahathaway · 2024-10-08T22:03:49Z

Please make sure that this flows into .NET 9.0. I believe fsharp's refs/heads/release/dev17.12 branch flows to .NET 9.0 release branch. Thanks in advance :)

@dotnet/fsharp - This change needs to be ported into .NET 9. What is the best way to get this backported to the fsharp's refs/heads/release/dev17.12 branch?

KevinRansom added 3 commits September 13, 2024 06:18

fix

924d799

temp

9f141fe

revert

da8df1f

KevinRansom requested a review from a team as a code owner September 13, 2024 16:48

vzarytovskii reviewed Sep 13, 2024

View reviewed changes

docs/release-notes/.Language/preview.md Outdated Show resolved Hide resolved

vzarytovskii and others added 3 commits September 13, 2024 19:12

Merge branch 'main' into fix17713

624935b

readme

8d78b7e

Merge branch 'fix17713' of https://github.com/KevinRansom/fsharp into…

a292814

… fix17713

KevinRansom added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Sep 13, 2024

KevinRansom enabled auto-merge (squash) September 14, 2024 01:01

Merge branch 'main' into fix17713

bafaf82

This comment was marked as outdated.

Sign in to view

This was referenced Sep 16, 2024

Additional nowarn directives in the generated code (part 2) fsprojects/FsLexYacc#221

Merged

Fix #17713 (sourcebuild phase 2 build error) #17748

Closed

Merge branch 'main' into fix17713

d2edd1d

T-Gro approved these changes Sep 16, 2024

View reviewed changes

Merge branch 'main' into fix17713

9d59467

KevinRansom disabled auto-merge September 16, 2024 20:21

KevinRansom merged commit 848c038 into dotnet:main Sep 16, 2024
32 checks passed

ellahathaway mentioned this pull request Oct 8, 2024

.NET Source-Build 9.0.100-rc.2.24474.1 October 2024 Updates dotnet/sdk#43993

Merged

This was referenced Oct 31, 2024

[Automated] PRs inserted in VS build main-35430.72 #17943

Closed

[Automated] PRs inserted in VS build feature.debugger.main-35430.275 #17944

Closed

dotnet-bot mentioned this pull request Nov 19, 2024

[Automated] PRs inserted in VS build feature.debugger.shadowDebug-35518.109 #18027

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix17713 - Reverting PR - 17649 - Make the interaction between #line and #nowarn directives consistent #17724

Fix17713 - Reverting PR - 17649 - Make the interaction between #line and #nowarn directives consistent #17724

KevinRansom commented Sep 13, 2024

github-actions bot commented Sep 13, 2024 •

edited

Loading

vzarytovskii commented Sep 13, 2024

Martin521 commented Sep 15, 2024 •

edited

Loading

KevinRansom commented Sep 16, 2024

Martin521 commented Sep 16, 2024 •

edited

Loading

This comment was marked as outdated.

Martin521 commented Sep 16, 2024 •

edited

Loading

Martin521 commented Sep 16, 2024 •

edited

Loading

KevinRansom commented Sep 16, 2024

Martin521 commented Sep 16, 2024

vzarytovskii commented Sep 16, 2024

ellahathaway commented Sep 16, 2024

ellahathaway commented Oct 8, 2024

Fix17713 - Reverting PR - 17649 - Make the interaction between #line and #nowarn directives consistent #17724

Fix17713 - Reverting PR - 17649 - Make the interaction between #line and #nowarn directives consistent #17724

Conversation

KevinRansom commented Sep 13, 2024

github-actions bot commented Sep 13, 2024 • edited Loading

⚠️ Release notes required, but author opted out

vzarytovskii commented Sep 13, 2024

Martin521 commented Sep 15, 2024 • edited Loading

KevinRansom commented Sep 16, 2024

Martin521 commented Sep 16, 2024 • edited Loading

This comment was marked as outdated.

Martin521 commented Sep 16, 2024 • edited Loading

Martin521 commented Sep 16, 2024 • edited Loading

KevinRansom commented Sep 16, 2024

Martin521 commented Sep 16, 2024

vzarytovskii commented Sep 16, 2024

ellahathaway commented Sep 16, 2024

ellahathaway commented Oct 8, 2024

github-actions bot commented Sep 13, 2024 •

edited

Loading

Martin521 commented Sep 15, 2024 •

edited

Loading

Martin521 commented Sep 16, 2024 •

edited

Loading

Martin521 commented Sep 16, 2024 •

edited

Loading

Martin521 commented Sep 16, 2024 •

edited

Loading